-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Scala3doc: Bugfixes related to symbol names and extension methods #10334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -168,7 +168,7 @@ case class TastyParser(qctx: QuoteContext, inspector: DokkaBaseTastyInspector, c | |||
|
|||
private def errorMsg[T](a: Any, m: => String, e: Throwable): Option[T] = | |||
val msg = try m catch case e: Throwable => a.toString | |||
println(s"ERROR: tree is faling: msg") | |||
println(s"ERROR: tree is faling: $msg") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic looks ok, I've asked for some stylistic changes
@@ -17,13 +17,13 @@ class ImplicitMembersExtensionTransformer(ctx: DokkaContext) extends Documentabl | |||
|
|||
def expandMember(outerMembers: Seq[Member])(c: Member): Member = | |||
val companion = c match | |||
case classlike: DClass => ClasslikeExtension.getFrom(classlike).flatMap(_.companion).flatMap(classlikeMap.get) | |||
case classlike: DClass => ClasslikeExtension.getFrom(classlike).flatMap(_.companion).map(classlikeMap) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change may not be safe (it will throw exception when classlikeMap
does not contains companion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to safe call (Map.get
) and log when classlikeMap is missing companion
@@ -248,6 +248,14 @@ class ScalaPageContentBuilder( | |||
|
|||
def signature(d: Documentable) = addChildren(signatureProvider.signature(d).asScala.toList) | |||
|
|||
private def buildSignature(d: Documentable, s: Signature) = signatureProvider.asInstanceOf[ScalaSignatureProvider].signature(d, s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can move signature
to ScalaSignatureProvider
object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using contentBuilder field from ScalaSignatureProvider class which will be hard to obtain in object
@@ -31,6 +31,11 @@ class ScalaSignatureProvider(contentConverter: CommentsToContentConverter, logge | |||
def driLink(text: String, dri: DRI): SignatureBuilder = ContentNodeBuilder(builder.driLink(text, dri)) | |||
} | |||
|
|||
def signature(d: Documentable, s: Signature) = signatureContent(d){ builder => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use Member
here?
bc30d09
to
424c093
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pikinier20 I fixed indentation & mixed brace/braceless style & whitespace issues for you.
I also removed the outdated code for handling extension names.
The PR LGTM now.
closes #10329
closes #227